Add the google-cas-issuer to the Google Marketplace release #4

maelvls · 2021-01-29T16:41:01Z

This PR is an attempt at adding https://github.com/jetstack/google-cas-issuer to the Google Marketplace release.

should I make sure to have the Prometheus annotations + service as part of this first iterations?
still not sure how to manually test that the Google Marketplace release works
should we create a default GoogleCASIssuer so that the user can start right away?

Signed-off-by: Maël Valais <[email protected]>

maelvls · 2021-02-01T11:21:23Z

Discussed this morning in the standup: should we create a default GoogleCASIssuer so that the user can start right away? @jakexks

Update: this PR does not have any default GoogleCASIssuer, but I did add a default namespace-scoped GoogleCASIssuer in #5

maelvls · 2021-02-03T18:52:55Z

Yay! I finally managed to manually test google-cas-issuer when deployed with mpdev install and it works perfectly!

Proof:

% kubectl describe cert demo-certificate
Events:
  Type    Reason     Age   From          Message
  ----    ------     ----  ----          -------
  Normal  Issuing    20s   cert-manager  Issuing certificate as Secret was previously issued by GoogleCASIssuer.cas-issuer.jetstack.io/googlecasissuer-sample
  Normal  Reused     20s   cert-manager  Reusing private key stored in existing Secret resource "demo-cert-tls"
  Normal  Requested  20s   cert-manager  Created new CertificateRequest resource "demo-certificate-v2rwr"
  Normal  Issuing    20s   cert-manager  The certificate has been successfully issued

⚠️ Caveat: I had to put the CRDs in ./templates/crds.yaml. Details in 9a4f70a:

As detailed in 1, CRDs that are applied through the CRD pre-install hook will not ever be updated or upgraded. The Helm documentation reads 4:

The resources that a hook creates are not tracked or managed as part of > the release. Once Tiller verifies that the hook has reached its ready > state, it will leave the hook resource alone. > > Practically speaking, this means that if you create resources in a hook, > you cannot rely upon helm delete to remove the resources. To destroy such > resources, you need to either write code to perform this operation in a > pre-delete or post-delete hook or add "helm.sh/hook-delete-policy" > annotation to the hook template file.

On top of that, it seems to be not possible (after multiple trials) to manage CRDs using Helm 3's crd-install hook using the crds/ folder 2. It seems like the only way would be to bundle crds inside the templates/ like cert-manager has been doing for a while.

Note that installing CRDs using the templates/ way also causes trouble. Rob Percival mentions in 1 that Helm has a problem with the CRD ordering 3 and that the issue has not been fixed yet, which means installing operators like google-cas-issuer breaks when the CRDs are inside templates/.

Final notes:

There is no test automation here, but will do them as part of Add smoke tests #5.
I added all the necessary documentation on how to test it manually

cc @jakexks

Signed-off-by: Maël Valais <[email protected]>

As detailed in [1], CRDs that are applied through the CRD pre-install hook will not ever be updated or upgraded. The Helm documentation reads [4]: > The resources that a hook creates are not tracked or managed as part of > the release. Once Tiller verifies that the hook has reached its ready > state, it will leave the hook resource alone. > > Practically speaking, this means that if you create resources in a hook, > you cannot rely upon helm delete to remove the resources. To destroy such > resources, you need to either write code to perform this operation in a > pre-delete or post-delete hook or add "helm.sh/hook-delete-policy" > annotation to the hook template file. On top of that, it seems to be not possible (after multiple trials) to manage CRDs using Helm 3's crd-install hook using the crds/ folder [2]. It seems like the only way would be to bundle crds inside the templates/ like cert-manager has been doing for a while. Note that installing CRDs using the templates/ way also causes trouble. Rob Percival mentions in [1] that Helm has a problem with the CRD ordering [3] and that the issue has not been fixed yet, which means installing operators like google-cas-issuer breaks when the CRDs are inside templates/. [1]: GoogleCloudPlatform/marketplace-k8s-app-tools#303 [2]: https://helm.sh/docs/developing_charts/#defining-a-crd-with-the-crd-install-hook [3]: helm/helm#2994 [4]: https://v2.helm.sh/docs/developing_charts/#hook-resources-are-not-managed-with-corresponding-releases Signed-off-by: Maël Valais <[email protected]>

Signed-off-by: Maël Valais <[email protected]>

wallrj · 2021-02-04T16:44:38Z

chart/jetstacksecure-mp/charts/google-cas-issuer/templates/crds.yaml

@@ -0,0 +1,246 @@
+{{ if .Values.installCRDs }}


That commit message is great. Worth pasting here for the benefit of future maintainers.

README.md

jakexks · 2021-02-08T16:36:41Z

README.md

+
+This will also verify the application using the [Google Cloud Marketplace verification tool](https://github.com/GoogleCloudPlatform/marketplace-k8s-app-tools/blob/c5899a928a2ac8d5022463c82823284a9e63b177/scripts/verify).
+
+[workload-identity]: https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity


I guess this goes further up the file where the missing link is?

chart/jetstacksecure-mp/charts/google-cas-issuer/templates/crds.yaml

jakexks · 2021-02-08T16:38:57Z

Only saw some minor text changes
/assign @maelvls

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Jake Sanders <[email protected]>

maelvls · 2021-02-08T18:36:36Z

/unassign

maelvls · 2021-02-09T16:11:38Z

/assign @jakexks
😅

jakexks · 2021-02-09T16:47:59Z

/lgtm tentative LGTM 😅

Signed-off-by: Maël Valais <[email protected]>

Signed-off-by: Richard Wall <[email protected]>

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

Signed-off-by: Richard Wall <[email protected]>

wallrj

Fixed a couple of cloud-build issues with @maelvls and successfully ran the cloud-build verify process.

/lgtm

wallrj · 2021-02-10T12:18:42Z

README.md


+> Note: although cert-manager's tags are of the form "v1.1.0", we chose to
+> use tags of the form "1.1.0" for the Google Marketplace for the sake of
+> consistency.


I don't think it's a choice. I think GCM enforces that the version be a semver without any prefix.

wallrj · 2021-02-10T13:22:25Z

cloudbuild.yaml

-        cat /scripts/dev > "/workspace/cmpt"
-        chmod +x /workspace/cmpt
+        cat /scripts/dev > "/workspace/mpdev"
+        chmod +x /workspace/mpdev


This breaks the following steps which still refer to cmpt

maelvls added 3 commits January 29, 2021 17:05

prettier the schema.yaml file

4009daf

Signed-off-by: Maël Valais <[email protected]>

google-cas-issuer: add chart

7231551

Signed-off-by: Maël Valais <[email protected]>

google-cas-issuer: forgot the CRDs

ac53c00

Signed-off-by: Maël Valais <[email protected]>

jetstack-bot added the size/XL label Jan 29, 2021

maelvls mentioned this pull request Feb 1, 2021

Draft #1

Closed

maelvls mentioned this pull request Feb 2, 2021

Add the google-cas-issuer helm chart cert-manager/google-cas-issuer#22

Closed

maelvls force-pushed the add-cas-issuer branch from af34d73 to c0b7fe5 Compare February 3, 2021 17:12

jetstack-bot added size/XXL and removed size/XL labels Feb 3, 2021

maelvls added 7 commits February 4, 2021 17:28

cloudbuild: use "mpdev" as in docs instead of "cmpt"

bee988b

Signed-off-by: Maël Valais <[email protected]>

fix google-cas-issuer tag (was v0.1.0 but expected 0.1.0)

a99d9d0

Signed-off-by: Maël Valais <[email protected]>

readme: reworked the steps

5ca712d

Signed-off-by: Maël Valais <[email protected]>

readme: detail how to use google-cas-issuer

e902ff1

Signed-off-by: Maël Valais <[email protected]>

readme: cleanup

e03cc2f

Signed-off-by: Maël Valais <[email protected]>

readme: rename namespace "test" to "test-ns" for less confusion

ad77ba6

Signed-off-by: Maël Valais <[email protected]>

maelvls force-pushed the add-cas-issuer branch from 5143586 to ad77ba6 Compare February 4, 2021 16:37

wallrj reviewed Feb 4, 2021

View reviewed changes

jakexks reviewed Feb 8, 2021

View reviewed changes

jetstack-bot assigned maelvls Feb 8, 2021

maelvls and others added 2 commits February 8, 2021 17:46

PR comments: fix link and minor changes

0e6c894

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Jake Sanders <[email protected]>

PR comment: missing POSIX EOF newline

a0b7b8c

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Jake Sanders <[email protected]>

jetstack-bot unassigned maelvls Feb 8, 2021

maelvls added this to the Beta milestone Feb 9, 2021

maelvls self-assigned this Feb 9, 2021

jetstack-bot assigned jakexks Feb 9, 2021

maelvls and others added 9 commits February 10, 2021 13:59

Add a NOTE.txt with a nice tutorial on how to get started

a4f44d3

Signed-off-by: Maël Valais <[email protected]>

Fix lingering references to cmpt with ./mpdev

7fa596d

Signed-off-by: Richard Wall <[email protected]>

Pull and push CAS issuer images

3f85f2b

Signed-off-by: Richard Wall <[email protected]>

Fix schema properties to correctly override Docker image registries

cea9428

Signed-off-by: Richard Wall <[email protected]>

PR comment: GCP account -> GKE cluster

37bb6be

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

PR comment: no need for a space after .Values

da7a947

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

Nicer Chart.yaml

32331f9

Signed-off-by: Maël Valais <[email protected]> Co-authored-by: Richard Wall <[email protected]>

whitespace

bee851a

Signed-off-by: Richard Wall <[email protected]>

Publish all the images with the same tag of "APP_VERSION" 1.0.0

daddec9

Signed-off-by: Richard Wall <[email protected]>

wallrj approved these changes Feb 10, 2021

View reviewed changes

jetstack-bot assigned wallrj Feb 10, 2021

jetstack-bot added the lgtm label Feb 10, 2021

jetstack-bot merged commit aeb2f31 into main Feb 10, 2021

maelvls modified the milestones: beta, initial-release Feb 11, 2021

maelvls deleted the add-cas-issuer branch November 3, 2021 09:40


		This will also verify the application using the [Google Cloud Marketplace verification tool](https://github.com/GoogleCloudPlatform/marketplace-k8s-app-tools/blob/c5899a928a2ac8d5022463c82823284a9e63b177/scripts/verify).

		[workload-identity]: https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity

Add the google-cas-issuer to the Google Marketplace release #4

Add the google-cas-issuer to the Google Marketplace release #4

Uh oh!

Conversation

maelvls commented Jan 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maelvls commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maelvls commented Feb 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wallrj Feb 4, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakexks Feb 8, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakexks commented Feb 8, 2021

Uh oh!

maelvls commented Feb 8, 2021

Uh oh!

maelvls commented Feb 9, 2021

Uh oh!

jakexks commented Feb 9, 2021

Uh oh!

wallrj left a comment

Choose a reason for hiding this comment

Uh oh!

wallrj Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

wallrj Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

maelvls commented Jan 29, 2021 •

edited

Loading

maelvls commented Feb 1, 2021 •

edited

Loading

maelvls commented Feb 3, 2021 •

edited

Loading